Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add "Log view" #17

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mongolyy
Copy link

Description

Add "Log view" at side bar.(like typescript playground)

@CLAassistant
Copy link

CLAassistant commented Mar 27, 2022

CLA assistant check
All committers have signed the CLA.

@g-plane
Copy link
Collaborator

g-plane commented Mar 27, 2022

This PR isn't just to add a feature about "Log View", however it's about allowing to execute code, which is misleading.

@mongolyy
Copy link
Author

Thank you for your comment!

This PR isn't just to add a feature about "Log View", however it's about allowing to execute code, which is misleading.

Should I stop executing code in the playground (in terms of security risks, etc.)?
If so, what is the best way to do this?
I'm terribly sorry, but I don't know a good way...

@mongolyy
Copy link
Author

I noticed your comment.
If execution itself is not a problem, I will use Function instead of eval()!
https://developer.mozilla.org/en-US/docs/web/javascript/reference/global_objects/eval#never_use_eval!

@g-plane
Copy link
Collaborator

g-plane commented Mar 27, 2022

You're right. Using eval (and Function, etc.) would be dangerous, though it will be run on clients. There's an alternative: Create a JavaScript module dynamically, then use import() to execute it, but it has a disadvantage: The code will and must be run under a module context, while users may want to run as a normal script, not a module.

There's a safer way to achieve this, but it's out of scope, because it's still under ECMAScript proposal (Stage 3).

I'm not sure whether we really need this feature or not, however it's nice to have. We don't need to close this PR. Let's leave it and see how others says. If it's highly requested, we can consider it.

@mongolyy
Copy link
Author

mongolyy commented Mar 27, 2022

Thank you very much for your detailed explanation.
Due to my lack of knowledge, I did not understand everything, but I did understand the outline.

Let's leave it and see how others says. If it's highly requested, we can consider it.

Ok! I agree with this approach.
ECMAScript proposal (perhaps shadowrealm ?) may also be in stage 4 when we get a response from others that they want it.

@g-plane
Copy link
Collaborator

g-plane commented Mar 27, 2022

Yes, it's ShadowRealm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants